Skip to content

MPDX-9894 - Prepare MPDX agent files for 4 axes review system - #1948

Merged
zweatshirt merged 2 commits into
mainfrom
MPDX-9894
Jul 29, 2026
Merged

MPDX-9894 - Prepare MPDX agent files for 4 axes review system#1948
zweatshirt merged 2 commits into
mainfrom
MPDX-9894

Conversation

@zweatshirt

@zweatshirt zweatshirt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

Must be merged with https://github.com/CruGlobal/claude-code-plugins/pull/25

The agent-review skill moved from a single 0–10 risk_score to a four-axis risk model (Blast Radius, Complexity, Sensitivity, Recoverability -> effort/danger). This PR updates the two places mpdx-react consumes that model so nothing shows stale data or reads the removed field.

Changes

Auto-approve workflow (ai-review-auto-approve.yml)

  • The review metadata no longer emits risk_score, so the approval comment/logs stopped showing a meaningful number. Now reads the axes object and reports danger N/6, effort N/6 instead.
  • No change to the approval gate... it still keys on risk_level (LOW/MEDIUM), blockers, and verdict. risk_score was only ever cosmetic here.

Code-review rules (.claude/rules/code-review.md)

  • Removed the ## Special Pattern Detection section since under the axis model its signals are now redundant (dependency/lockfile changes are covered by universal signals; next.config/cache/missing-test cases are already handled by Critical patterns and Agent Triggers).
  • Kept the two signals that were genuinely unique (codegen must run clean; new REST-proxy .graphql needs a matching resolver/datahandler) by moving them into Data Integrity Focus Areas as review guidance.
  • Reworded the tier descriptions so they just classify files and let the skill own the tier→axis mapping — so the rules file won't drift when the model changes again.

Testing

N/A

  • Double check behavior post merge.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
  • I have run the Claude Code /quality:agent-review command locally and fixed any relevant suggestions
  • I have requested a review from another person on the project
  • I have tested my changes in preview or in staging
  • I have cleaned up my commit history

@github-actions

Copy link
Copy Markdown
Contributor

Bundle sizes [mpdx-react]

Compared against 0a0f7c3

No significant changes found

@zweatshirt
zweatshirt marked this pull request as ready for review July 28, 2026 20:38
@zweatshirt zweatshirt self-assigned this Jul 28, 2026

@zweatshirt zweatshirt left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Multi-Agent Code Review — Verdict: ✅ CLEAN (0 blockers)

3 agents (Security, Standards, Architecture) · standard mode · Risk: 🟠 HIGH · auto-approval not triggered (review-process change → human review required)

This PR is correctly-scoped, contract-accurate prep for the 4-axis review model. Reviewed against its companion PR CruGlobal/claude-code-plugins#25, which ports the axis model into the production agent-review skill — the two are a designed pair.

The two PRs are a matched migration

Contract element Old skill (installed) New skill (plugins#25) This workflow change
Metadata emits risk_score, no axes drops risk_score, emits axes:{danger,effort,…} reads axes.danger/axes.effort, drops risk_score
Gate key risk_level risk_level (derived from danger) risk_level (unchanged) ✅

plugins#25 explicitly names this as the sanctioned path: "…or update the auto-approve workflow, which still reads risk_score." Removing risk_score here is required — the new skill stops emitting it.

Why deleting ## Special Pattern Detection is correct cleanup (not a weakening)

The new axis model subsumes every deleted modifier as a universal default, and handles the key cases more strictly:

  • yarn.lock / lockfile drift → new model: "Any dependency-manifest edit or lockfile-only change → Sensitivity ≥ 2, Recoverability ≥ 1 (danger ≥ 3 → HIGH)" — caught universally, where the deleted repo rule gave only +1.
  • Dependency bumps (any package) → danger ≥ 3 → HIGH → not auto-approvable, making the old repo-specific "critical package" list redundant.
  • cache.ts typePolicies / next.config CSP → both are Critical-tier files, which the new model maps to Sensitivity ≥ 2 automatically.

The tier heading names are correctly preserved (the new model parses those exact strings), and the new prose "how each tier maps onto the risk axes is defined by the skill" is now accurate — plugins#25 implements exactly that mapping.

What's clean

  • ✅ All four auto-approve gates (risk_level, blockers, verdict, review-id dedup) untouched; risk_score was display-only.
  • ✅ No permission / secret / trigger-abuse regression in the workflow.
  • axes parsing is null-safe (axes && typeof axes === 'object', danger ?? '?') — degrades gracefully to ?/6 on old-format metadata.
  • ✅ Debt reduced — the rules file no longer duplicates the skill's scoring weights; the skill is now the single source of truth.

Risk = HIGH (confirmed by the new rubric itself)

Both changed files match .claude/rules/code-review* and .github/workflows/*auto* — anchored at Sensitivity = 3 ("the review/CI process itself") in the very model this PR introduces. With Recoverability ~0–1, danger = 3–4 → HIGH/CRITICAL. The pipeline's own rubric agrees: a change to the review process warrants human review and is never auto-approvable.

The one actionable item (operational, not code)

🔵 Deployment sequencing. If this PR merges while reviewers still run the old plugin (v1.6.0), approval comments briefly render danger ?/6, effort ?/6 (null-safe, gates unaffected) and point-model scoring loses the now-redundant repo-specific modifiers. Both vanish once the cru-plugins/quality upgrade from plugins#25 is rolled out. Recommendation: land/deploy plugins#25 together with this PR (or first). No code change needed — the artifacts are correctly decoupled and degrade gracefully in every ordering.

Summary

Agent Blockers Informational notes Confidence
Security 0 2 High
Standards 0 3 High
Architecture 0 2 High

Nothing to fix in the code. Only coordinate the merge/deploy with plugins#25.

🤖 Multi-agent review via /quality:agent-review (standard mode). Findings are informational; verdict CLEAN. Reply /dismiss: <reason> to any comment to acknowledge it.

- `pages/api/Schema/**/*.{ts,graphql}` — REST-proxy resolvers, schemas, and data handlers (silent data-reshaping risk)
- `pages/api/Schema/**/datahandler*.ts` — response transformation feeding the REST proxy
- `pages/api/**/*.page.ts` (excluding `auth/` and `graphql-rest` — already Critical) — other API/lambda routes
- `src/components/**/*.graphql` — GraphQL operations (cache normalization and pagination correctness)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are component GraphQL files actually high-risk?

Suggested change

@canac

canac commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

I'm excited to try this out!

@zweatshirt
zweatshirt merged commit 20aa7a9 into main Jul 29, 2026
24 checks passed
@zweatshirt
zweatshirt deleted the MPDX-9894 branch July 29, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants